Use a webview for WP.com login #21414

jkmassel · 2024-11-04T22:57:08Z

Fixes a fairly common login issue reported in https://a8c.slack.com/archives/C02AVAR9B/p1728576885606729.

To Test:

Install both Jetpack and WordPress.
Try logging into WP.com
Note that it works

Regression Notes

Potential unintended areas of impact
Could be other issues around login – I tried very narrowly address the issue in this PR – nothing has been removed, so everything should work like it did before (for instance, use re-authentication).
What I did to test those areas of impact (or what existing automated tests I relied on)
n/a
What automated tests I added (or what prevented me from doing so)
There wouldn't be a lot of benefit to automated tests at this point.

PR Submission Checklist:

I have completed the Regression Notes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing Checklist (strike-out the not-applying and unnecessary ones):

dangermattic · 2024-11-04T22:59:21Z

	5 Warnings
⚠️	Class `WPcomLoginHelper` is missing tests, but `unit-tests-exemption` label was set to ignore this.
⚠️	Class `ServiceConnection` is missing tests, but `unit-tests-exemption` label was set to ignore this.
⚠️	Class `WPcomLoginClient` is missing tests, but `unit-tests-exemption` label was set to ignore this.
⚠️	Class `WPcomLoginError` is missing tests, but `unit-tests-exemption` label was set to ignore this.
⚠️	This PR is assigned to the milestone 25.7. The due date for this milestone has already passed. Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

wpmobilebot · 2024-11-04T23:16:09Z

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr21414-355774f
	Commit	`355774f`
	Direct Download	`wordpress-prototype-build-pr21414-355774f.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2024-11-04T23:17:17Z

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr21414-355774f
	Commit	`355774f`
	Direct Download	`jetpack-prototype-build-pr21414-355774f.apk`

Note: Google Login is not supported on these builds.

crazytonyli · 2024-11-04T23:42:28Z

WordPress/src/main/java/org/wordpress/android/ui/accounts/login/WPcomLoginHelper.kt

+
+        runBlocking {
+            val tokenResult = loginClient.exchangeAuthCodeForToken(code, BuildConfig.OAUTH_REDIRECT_URI)
+            accountStore.updateAccessToken(tokenResult.getOrThrow())


I think you'd want to check if the token belongs to the current user before saving it. Users can log in with any account from the login webpage.

This should be for first-time authentication only – I'm thinking we'll want a reauthenticate method that does what you say – forces login to a specific account

nbradbury · 2024-11-05T13:16:32Z

@jkmassel I'm running into some issues with this. When I try to login with a 2FA account, I see this screen and can't go any further:

If I try with a non-2FA account, after I login I'm not redirected to the app - instead I'm still in the browser. Is this intended? I also did not expect to be taken to the browser to login. Is it not possible to do this in a WebView within the app?

wplogin.mp4

nbradbury · 2024-11-05T13:17:58Z

WordPress/src/main/AndroidManifest.xml

@@ -113,7 +113,18 @@
        <activity
            android:name=".ui.accounts.LoginActivity"
            android:theme="@style/LoginTheme.TransparentSystemBars"
-            android:windowSoftInputMode="adjustResize" />
+            android:windowSoftInputMode="adjustResize"
+            android:exported="true">


Sonarcloud flags this as a security risk. Does it need to be exported?

My understanding is that this was required in order to handle custom URL schemes, but I might be incorrect!

Ah, I suspect you're right about that. This Sonarcloud page mentions ways to fix this warning.

jkmassel · 2024-11-05T23:23:54Z

If I try with a non-2FA account, after I login I'm not redirected to the app - instead I'm still in the browser. Is this intended?

That's really odd – I wonder if maybe the app needs to be fully uninstalled? You're being redirected to http://android.a8c.com, which is the old behaviour 🤔

Is it not possible to do this in a WebView within the app

Good question! I had assumed that we'd want to use the default browser for password managers, etc – if we can do it in an in-app browser that would be nicer for sure.

nbradbury · 2024-11-06T12:36:35Z

That's really odd – I wonder if maybe the app needs to be fully uninstalled? You're being redirected to http://android.a8c.com, which is the old behaviour

I tried again, this time using a new emulator instance (so no previous install), and I'm still seeing this behavior.

wpmobilebot · 2024-11-18T21:59:48Z

Project dependencies changes

The following changes in project dependencies were detected (configuration wordpressVanillaReleaseRuntimeClasspath):

list

tree

+\--- androidx.browser:browser:1.5.0 -> 1.8.0 (*)

jkmassel · 2024-11-18T22:08:38Z

@nbradbury – could you give this a try again? It now uses Chrome Tabs in-app, which should share all of password management goodness we need.

I tried it with a Google-synced passkey and it worked, and I tried with a Yubikey and that worked too (on my ancient Galaxy S9) so I'm hopeful it works for you too!

nbradbury · 2024-11-19T12:23:10Z

could you give this a try again? It now uses Chrome Tabs in-app, which should share all of password management goodness we need.

@jkmassel Chrome Custom tabs is a much better solution, but I'm still seeing issues. With a non-2FA account, I end up being redirected to the Automattic home page.

redirect.mp4

With a 2FA account, I still get stuck in some sort of passkey loop which I don't understand.

passkey.mp4

nbradbury · 2024-11-19T14:17:12Z

gradle/libs.versions.toml

@@ -9,6 +9,7 @@ androidx-activity = '1.9.3'
 androidx-annotation = '1.9.1'
 androidx-appcompat = '1.7.0'
 androidx-arch-core = '2.2.0'
+androidx-browser = '1.5.0'


Note that this is an outdated version of the library. The latest is 1.8.0.

nbradbury · 2024-11-19T14:21:33Z

WordPress/src/main/java/org/wordpress/android/ui/accounts/LoginActivity.java

+    public void showEmailLoginScreen(@NonNull Context context) {
+        CustomTabsIntent intent = new CustomTabsIntent.Builder()
+                .setShareState(CustomTabsIntent.SHARE_STATE_OFF)
+                .setStartAnimations(this, R.anim.activity_slide_up_from_bottom, R.anim.activity_slide_up_from_bottom)


For consistency with the other login screens, WDYT about changing this to slide in from the right, slide out to the left?

It’s not a secret

wpmobilebot · 2024-12-05T12:07:19Z

Version 25.5 has now entered code-freeze, so the milestone of this PR has been updated to 25.6.

sonarqubecloud · 2024-12-16T14:01:01Z

Quality Gate failed

Failed conditions
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

codecov · 2024-12-16T14:15:54Z

Codecov Report

Attention: Patch coverage is 2.35294% with 83 lines in your changes missing coverage. Please review.

Project coverage is 39.38%. Comparing base (34c994a) to head (355774f).

Files with missing lines	Patch %	Lines
...droid/fluxc/network/rest/wpapi/WPcomLoginClient.kt	0.00%	41 Missing ⚠️
...ress/android/ui/accounts/login/WPcomLoginHelper.kt	0.00%	29 Missing ⚠️
...icationpasswords/WPcomAuthorizationCodeResponse.kt	0.00%	6 Missing ⚠️
...roid/fluxc/network/rest/wpcom/auth/AppSecrets.java	0.00%	3 Missing ⚠️
...rg/wordpress/android/fluxc/store/AccountStore.java	0.00%	2 Missing ⚠️
...press/android/ui/accounts/LoginNavigationEvents.kt	0.00%	1 Missing ⚠️
...rdpress/android/ui/accounts/UnifiedLoginTracker.kt	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk   #21414      +/-   ##
==========================================
- Coverage   39.44%   39.38%   -0.06%     
==========================================
  Files        2119     2125       +6     
  Lines       99556    99711     +155     
  Branches    15313    15333      +20     
==========================================
+ Hits        39269    39273       +4     
- Misses      56806    56957     +151     
  Partials     3481     3481

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nbradbury · 2024-12-16T15:35:44Z

@jkmassel I continue to see this passkey issue when trying to login using an account with 2FA. I'm not sure what this is about but we'll definitely want to address this before merging.

passkey.mp4

wpmobilebot · 2024-12-16T15:53:10Z

Version 25.6 has now entered code-freeze, so the milestone of this PR has been updated to 25.7.

AdamGrzybkowski · 2024-12-23T11:07:54Z

I continue to see this passkey issue when trying to login using an account with 2FA. I'm not sure what this is about but we'll definitely want to address this before merging.

@nbradbury What do you use for Passkeys? There's a flag in Chrome that needs to be enabled to make it work with 1Password. There's info about this here.

AdamGrzybkowski · 2024-12-23T11:32:01Z

WordPress/src/main/AndroidManifest.xml

+            android:windowSoftInputMode="adjustResize"
+            android:exported="true">


I think you're missing android:launchMode="singleTask". Without that, a new Activity is created after the OAuth flow is approved, and you can navigate back to it (that shouldn't be possible).

jetpack-launchmode.mp4

singleTop does work as well in most browsers except Firefox. So singleTask is the safe option here.

Once you add that you should handle the onNewIntent properly.

AdamGrzybkowski · 2024-12-23T11:36:41Z

WordPress/src/main/java/org/wordpress/android/ui/accounts/LoginActivity.java

+        CustomTabsIntent intent = new CustomTabsIntent.Builder()
+                .setShareState(CustomTabsIntent.SHARE_STATE_OFF)
+                .setStartAnimations(this, R.anim.activity_slide_in_from_right, R.anim.activity_slide_out_to_left)
+                .setExitAnimations(this, R.anim.activity_slide_in_from_left, R.anim.activity_slide_out_to_right)
+                .setUrlBarHidingEnabled(true)
+                .setInstantAppsEnabled(false)
+                .setShowTitle(false)
+                .build();


Sorry for jumping in, but since I took a look I wanted to share.

Using Custom Tabs with the magic link leads to a weird experience. Assuming you are not logged into WP in the web browser, you will have to open the email client to get the magic link. Tapping this magic link opens the web browser, not the Custom Tab which was confusing slightly. In the end, it worked, however, 3 apps were required to finish the process.

AdamGrzybkowski · 2024-12-23T11:39:54Z

libs/fluxc/src/main/java/org/wordpress/android/fluxc/network/rest/wpcom/auth/AppSecrets.java

        mAppId = appId;
        mAppSecret = appSecret;
+        mRedirectUri = redirectUri;


@nbradbury Have you run the bundle exec fastlane run configure_apply command? New secrets were added - redirectUri so you might be missing it. The OAuth flow won't work without those, the redirect URL that's expected is likely null in your case, so a web browser takes care of it, because it's not the one that's set in the AndroidManifest.xml.

jkmassel requested review from nbradbury and oguzkocer November 4, 2024 22:57

crazytonyli reviewed Nov 4, 2024

View reviewed changes

nbradbury reviewed Nov 5, 2024

View reviewed changes

jkmassel force-pushed the use/webview-for-dotcom-login branch from eb5c5a5 to d93b1f8 Compare November 18, 2024 22:07

jkmassel requested a review from nbradbury November 18, 2024 22:08

nbradbury reviewed Nov 19, 2024

View reviewed changes

jkmassel force-pushed the use/webview-for-dotcom-login branch from e8ffbf9 to bd142eb Compare November 27, 2024 23:35

jkmassel requested a review from nbradbury November 27, 2024 23:38

jkmassel marked this pull request as ready for review November 27, 2024 23:38

jkmassel added the unit-tests-exemption label Nov 27, 2024

jkmassel added this to the 25.5 milestone Nov 27, 2024

jkmassel force-pushed the use/webview-for-dotcom-login branch from bd142eb to 917cb9b Compare November 27, 2024 23:44

jkmassel and others added 7 commits November 27, 2024 17:19

Use a webview for WP.com login

d8b9ea1

Use Custom Chrome Tabs

eb3d0d9

Rename showEmailLoginScreen->showWPcomLoginScreen

e2e9e16

Use showWPcomLoginScreen to reauthenticate

8c8a89d

Don’t compile if secrets are missing

ed0fa69

Use the latest secrets.properties

2ab83b1

Changed start and exit animations

2824d28

jkmassel added 3 commits November 27, 2024 17:19

Preload the login page

f315a0d

Fix Detekt issues

921abfe

Move redirect URI into build.grade

0fbfecd

It’s not a secret

jkmassel force-pushed the use/webview-for-dotcom-login branch from ebe5b84 to 0fbfecd Compare November 28, 2024 00:19

wpmobilebot modified the milestones: 25.5, 25.6 Dec 5, 2024

Merge branch 'trunk' into use/webview-for-dotcom-login

355774f

wpmobilebot modified the milestones: 25.6, 25.7 Dec 16, 2024

AdamGrzybkowski reviewed Dec 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a webview for WP.com login #21414

Use a webview for WP.com login #21414

jkmassel commented Nov 4, 2024

dangermattic commented Nov 4, 2024 •

edited

Loading

wpmobilebot commented Nov 4, 2024 •

edited

Loading

wpmobilebot commented Nov 4, 2024 •

edited

Loading

crazytonyli Nov 4, 2024

jkmassel Nov 5, 2024

nbradbury commented Nov 5, 2024 •

edited

Loading

nbradbury Nov 5, 2024

jkmassel Nov 5, 2024

nbradbury Nov 6, 2024

jkmassel commented Nov 5, 2024

nbradbury commented Nov 6, 2024

wpmobilebot commented Nov 18, 2024

jkmassel commented Nov 18, 2024

nbradbury commented Nov 19, 2024

nbradbury Nov 19, 2024

nbradbury Nov 19, 2024 •

edited

Loading

wpmobilebot commented Dec 5, 2024

sonarqubecloud bot commented Dec 16, 2024

codecov bot commented Dec 16, 2024

nbradbury commented Dec 16, 2024

wpmobilebot commented Dec 16, 2024

AdamGrzybkowski commented Dec 23, 2024 •

edited

Loading

AdamGrzybkowski Dec 23, 2024

AdamGrzybkowski Dec 23, 2024

AdamGrzybkowski Dec 23, 2024

		android:windowSoftInputMode="adjustResize"
		android:exported="true">

Use a webview for WP.com login #21414

Are you sure you want to change the base?

Use a webview for WP.com login #21414

Conversation

jkmassel commented Nov 4, 2024

To Test:

Regression Notes

PR Submission Checklist:

Testing Checklist (strike-out the not-applying and unnecessary ones):

dangermattic commented Nov 4, 2024 • edited Loading

wpmobilebot commented Nov 4, 2024 • edited Loading

wpmobilebot commented Nov 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nbradbury commented Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkmassel commented Nov 5, 2024

nbradbury commented Nov 6, 2024

wpmobilebot commented Nov 18, 2024

Project dependencies changes

jkmassel commented Nov 18, 2024

nbradbury commented Nov 19, 2024

Choose a reason for hiding this comment

nbradbury Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

wpmobilebot commented Dec 5, 2024

sonarqubecloud bot commented Dec 16, 2024

Quality Gate failed

codecov bot commented Dec 16, 2024

Codecov Report

nbradbury commented Dec 16, 2024

wpmobilebot commented Dec 16, 2024

AdamGrzybkowski commented Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dangermattic commented Nov 4, 2024 •

edited

Loading

wpmobilebot commented Nov 4, 2024 •

edited

Loading

wpmobilebot commented Nov 4, 2024 •

edited

Loading

nbradbury commented Nov 5, 2024 •

edited

Loading

nbradbury Nov 19, 2024 •

edited

Loading

AdamGrzybkowski commented Dec 23, 2024 •

edited

Loading